feat(dev): Upgrade kafka and zookeeper to support Apple M1 #28574

armenzg · 2021-09-14T18:28:21Z

This version of zookeper does not throw qemu panics on Apple's M1.

See issue: confluentinc/kafka-images#80 (comment)

armenzg · 2021-09-14T18:44:45Z

.github/actions/setup-sentry/action.yml

@@ -162,7 +162,7 @@ runs:
            --name sentry_zookeeper \
            -d --network host \
            -e ZOOKEEPER_CLIENT_PORT=2181 \
-            confluentinc/cp-zookeeper:4.1.0


It seems the CI was running an older version.

What version do we run in production?

I believe we run 4.1.2-3 and version 3.4.11 for zookeeper.

Should I make the CI test against those versions?

As curiosity, we run a range of versions across the org: https://github.com/search?q=org%3Agetsentry+cp-kafka&type=code

Yeah I think we should have CI match production, let's leave comments here

@billyvg I've discovered that version of zookeeper we use in production is no longer available on Docker hub, thus, we cannot match the versions between CI and production.

FYI, this part of the PR is completely unnecessary to me.

Asked about the versions picked in #20543 (review)

armenzg · 2021-09-14T18:55:17Z

I tried getting rid of zookeeper but it got complicated.

billyvg

We should also change these in getsentry

billyvg · 2021-09-14T19:40:11Z

.github/actions/setup-sentry/action.yml

@@ -162,7 +162,7 @@ runs:
            --name sentry_zookeeper \
            -d --network host \
            -e ZOOKEEPER_CLIENT_PORT=2181 \
-            confluentinc/cp-zookeeper:4.1.0


Yeah I think we should have CI match production, let's leave comments here

billyvg · 2021-09-14T19:40:33Z

src/sentry/conf/server.py

@@ -1724,15 +1724,15 @@ def build_cdc_postgres_init_db_volume(settings):
    ),
    "zookeeper": lambda settings, options: (
        {
-            "image": "confluentinc/cp-zookeeper:5.1.2",
+            "image": "confluentinc/cp-zookeeper:6.2.0",


Can we leave comments here about the versions and why it differs from CI/prod

armenzg · 2021-09-15T12:48:44Z

We should also change these in getsentry

There's no changes required on getsentry as far as I know.

avgupta456 · 2021-09-17T21:19:34Z

When developing locally with Docker today, I ran into a new issue of sentry_kafka and sentry_zookeeper docker containers continually crashing and restarting. The error logs match confluentinc/cp-docker-images#883. I believe this is due to updating to 6.2.0 from 5.1.2 in conf/server.py. Reverting these two lines solves the issue locally.

armenzg · 2021-09-20T12:54:08Z

If this doesn't get reverted today, I will look into fixing it tomorrow.

Are there any tests that should have caught this? What tests could catch such a regression?

…28574)" This reverts commit b4bfb00. It seems to cause the images to restart (see [comment](#28574 (comment))).

armenzg · 2021-09-20T12:57:18Z

I created #28672 in case someone wants to revert this change while I'm off today.

When we [upgraded the kafka and zookeeper images we use][1], we did not update the volume to use the full path (as the [instructions indicate][2]), thus, started to see this error: > Command [/usr/local/bin/dub path /var/lib/kafka/data writable] FAILED ! This change adds the full path. [1]: #28574 [2]: https://docs.confluent.io/platform/current/installation/docker/operations/external-volumes.html#data-volumes-for-kafka-and-zk

armenzg · 2021-09-21T13:59:21Z

There's a fix in #28724

When we [upgraded the kafka and zookeeper images we use][1], we did not update the volume to use the full path (as the [instructions indicate][2]), thus, started to see this error: > Command [/usr/local/bin/dub path /var/lib/kafka/data writable] FAILED ! This change adds the full path. [1]: #28574 [2]: https://docs.confluent.io/platform/current/installation/docker/operations/external-volumes.html#data-volumes-for-kafka-and-zk

…28574)" (#28672) This reverts commit b4bfb00. Originally (see [originally reported issue](#28574 (comment))), instead of reverting my change, I landed a fix that only seems to work on Apple M1 (see fix #28724). Nevertheless, It seems that the *Intel* images would still fail with the same error (see issue #29022) with: > Command [/usr/local/bin/dub path /var/lib/kafka/data writable] FAILED ! Let's revert it and I will try again later. Fixes #29022

feat(ci): Upgrade kafka and zookeeper

71e8c82

armenzg added the Component: Developer Environment This covers issues related to setting up a developer's environment label Sep 14, 2021

armenzg added this to the Apple M1 devservices support milestone Sep 14, 2021

armenzg self-assigned this Sep 14, 2021

armenzg commented Sep 14, 2021

View reviewed changes

armenzg marked this pull request as ready for review September 14, 2021 18:53

armenzg requested a review from a team as a code owner September 14, 2021 18:53

billyvg reviewed Sep 14, 2021

View reviewed changes

armenzg added 2 commits September 15, 2021 08:10

Merge branch 'master' into armenzg/dev/upgrade-kafka-zookeeper

c60a0d9

Match to production versions

eb8c165

vercel bot deployed to Preview – sentry September 15, 2021 12:21 View deployment

vercel bot deployed to Preview – storybook September 15, 2021 12:21 View deployment

Update version

9cdb788

vercel bot deployed to Preview – sentry September 15, 2021 12:44 View deployment

vercel bot deployed to Preview – storybook September 15, 2021 12:44 View deployment

Fix comment location

8edf6e9

vercel bot deployed to Preview – sentry September 15, 2021 12:52 View deployment

vercel bot deployed to Preview – storybook September 15, 2021 12:52 View deployment

Update comment

25ffa5d

vercel bot deployed to Preview – sentry September 15, 2021 13:09 View deployment

vercel bot deployed to Preview – storybook September 15, 2021 13:09 View deployment

armenzg requested a review from billyvg September 15, 2021 13:09

billyvg approved these changes Sep 15, 2021

View reviewed changes

armenzg changed the title ~~feat(ci): Upgrade kafka and zookeeper~~ feat(dev): Upgrade kafka and zookeeper to support Apple M1 Sep 15, 2021

armenzg merged commit b4bfb00 into master Sep 15, 2021

armenzg deleted the armenzg/dev/upgrade-kafka-zookeeper branch September 15, 2021 13:57

armenzg added a commit that referenced this pull request Sep 20, 2021

Revert "feat(dev): Upgrade kafka and zookeeper to support Apple M1 (#…

e2d75c9

…28574)" This reverts commit b4bfb00. It seems to cause the images to restart (see [comment](#28574 (comment))).

armenzg mentioned this pull request Sep 20, 2021

Revert "feat(dev): Upgrade kafka and zookeeper to support Apple M1 (#… #28672

Merged

armenzg mentioned this pull request Sep 21, 2021

fix(dev): Use full path for Kafka container #28724

Merged

armenzg linked an issue Sep 21, 2021 that may be closed by this pull request

Zookeper Docker image not working on Apple's arm64 #28570

Closed

armenzg mentioned this pull request Oct 6, 2021

feat(dev): Use newer kafka & zookeeper images to support Apple arm64 #29084

Merged

github-actions bot locked and limited conversation to collaborators Oct 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dev): Upgrade kafka and zookeeper to support Apple M1 #28574

feat(dev): Upgrade kafka and zookeeper to support Apple M1 #28574

armenzg commented Sep 14, 2021 •

edited

Loading

armenzg Sep 14, 2021

billyvg Sep 14, 2021

armenzg Sep 14, 2021

billyvg Sep 14, 2021

armenzg Sep 15, 2021

armenzg Sep 15, 2021

armenzg commented Sep 14, 2021

billyvg left a comment

billyvg Sep 14, 2021

billyvg Sep 14, 2021

armenzg Sep 15, 2021

armenzg commented Sep 15, 2021

avgupta456 commented Sep 17, 2021

armenzg commented Sep 20, 2021

armenzg commented Sep 20, 2021

armenzg commented Sep 21, 2021

feat(dev): Upgrade kafka and zookeeper to support Apple M1 #28574

feat(dev): Upgrade kafka and zookeeper to support Apple M1 #28574

Conversation

armenzg commented Sep 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

armenzg commented Sep 14, 2021

billyvg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

armenzg commented Sep 15, 2021

avgupta456 commented Sep 17, 2021

armenzg commented Sep 20, 2021

armenzg commented Sep 20, 2021

armenzg commented Sep 21, 2021

armenzg commented Sep 14, 2021 •

edited

Loading